Reviewable status: 0 of 17 files reviewed, 4 unresolved discussions (waiting on @rablador)

ios/MullvadVPNUITests/Pages/EditCustomListLocationsPage.swift line 61 at r1 (raw file):

        }

        app.navigationBars[navigationBarName].buttons.firstMatch.tap()

This class EditCustomListLocationsPage is now for both add and edit, so the action context is required. Maybe it should be broken out into two page classes? There are a few differences between the edit and add views even though they also have much in common.

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 37 at r1 (raw file):

    }

    func testTakeScreenshotOfSecuredConnection() throws {

Should this screenshot still be captured? My interpretation of https://linear.app/mullvad/issue/IOS-684/update-screenshots-to-use-pq-tunnels is that this screenshot should be replaced by the screenshot with a quantum secured connection. It's nice to still have this around, but the drawback is that it opens up for possible mistakes if all screenshots should be used except this one.

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 112 at r1 (raw file):

        CustomListPage(app)
            .renameCustomList(name: "Low latency locations")

nit: would be nice as a constant since it is reused

ios/TestPlans/MullvadVPNScreenshots.xctestplan line 31 at r1 (raw file):

        "SettingsMigrationTests",
        "SettingsTests"
      ],

Make sure to also uncheck "Automatically include new tests" for the ScreenshotTests test plan, otherwise new tests will be included in the plan.

Reviewable status: 0 of 18 files reviewed, 3 unresolved discussions (waiting on @niklasberglund)

ios/MullvadVPNUITests/Pages/EditCustomListLocationsPage.swift line 61 at r1 (raw file):

Right now the difference is basically the navigation title. We don't use inheritance much in the app, but I think a new AddCustomListLocationsPage could simply inherit EditCustomListLocationsPage and override the back tap function.

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 37 at r1 (raw file):

Yeah, you're probably right. I'll remove this test.

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 112 at r1 (raw file):

Yep

ios/TestPlans/MullvadVPNScreenshots.xctestplan line 31 at r1 (raw file):

Done.

Reviewable status: 0 of 18 files reviewed, 3 unresolved discussions

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @rablador)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

        if TunnelControlPage(app).connectionIsSecured {
            TunnelControlPage(app)
                .tapDisconnectButton()

I think it would be easier to follow the code if this reset is done in testTakeScreenshotOfQuantumSecuredConnection teardown.

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @niklasberglund)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

I agree, but that also requires each test to go back to the home screen when they're done. On startup that's where we begin each time. The real "problem" here is that the tests aren't completely stand-alone, but it felt unnecessarily thorough/slow to uninstall the app between each of them.

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @rablador)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

It looks like testTakeScreenshotOfQuantumSecuredConnection is the only test case connecting to a relay, so this code will only run after testTakeScreenshotOfQuantumSecuredConnection, so it's kind of its teardown but instead it is in the next test cases' setup.

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions (waiting on @niklasberglund)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 33 at r2 (raw file):

Discussed offline. Will inherit from LoggedInWithTimeUITestCase to make tests independent of each other (results in a logout if logged in before each test).

Reviewable status: 0 of 18 files reviewed, 4 unresolved discussions

Reviewed 12 of 17 files at r1, 5 of 6 files at r2, all commit messages.
Reviewable status: 17 of 18 files reviewed, 5 unresolved discussions (waiting on @niklasberglund and @rablador)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 25 at r3 (raw file):

    func testTakeScreenshotOfQuantumSecuredConnection() throws {
        // We can't close banners in the screenshot tests due to how the NotificationController view

Is it because the notification is not accessible ?
Can we make it accessible in that case ? That seems like a silly limitation to have.

Reviewable status: 17 of 18 files reviewed, 5 unresolved discussions (waiting on @buggmagnet and @niklasberglund)

ios/MullvadVPNUITests/Screenshots/ScreenshotTests.swift line 25 at r3 (raw file):

It's indeed silly. There's a PR for it here: https://linear.app/mullvad/issue/IOS-704/make-notifications-accessible

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @niklasberglund)